Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Try to make #changeToNewRole a bit more reliable #1013

Closed
wants to merge 1 commit into from
Closed

Try to make #changeToNewRole a bit more reliable #1013

wants to merge 1 commit into from

Conversation

brbrr
Copy link
Contributor

@brbrr brbrr commented Mar 8, 2018 •
edited
Loading

There are 2 buttons with the same locators in the EditTeamMemberPage. 'Update' button is in disabled state it role was not changed.

This PR makes #changeToNewRole click only on active 'Update' button, also it chains the driver-related calls to guarantee synchronous execution

@brbrr brbrr self-assigned this Mar 8, 2018
alisterscott
alisterscott reviewed Mar 8, 2018
View reviewed changes
Copy link
Contributor

@alisterscott alisterscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually any different than what was there before?

I don't really understand why you'd need to chain these actions here?

@brbrr
Copy link
Contributor Author

brbrr commented Mar 8, 2018

It is different :)

So while I tried to debug CircleCI failures I noticed that Invites test fails after that method call (https://github.com/Automattic/wp-e2e-tests/blob/master/specs/wp-invite-users-spec.js#L706)
Not sure why, but the local run was clicking on the disabled button(before it selects 'Author' from dropdown)

So here I changed button locator since there 2 buttons located with button.form-button.is-primary and it should click only not disabled button.

I don't really understand why you'd need to chain these actions here?

In my understanding, all(or most) of the driver and driverHelper methods return a promise. And in my experience, WebDriverJS promise manager isn't reliable enough, that's why I prefer to chain all these promise calls whenever it's needed or not. As a side benefit - it would be much easier to convert then chains into await calls when we'll move to async/await style (#329)

@brbrr brbrr changed the title make #changeToNewRole a bit reliable Try to make #changeToNewRole a bit more reliable Mar 8, 2018
@brbrr brbrr closed this Mar 8, 2018
@brbrr
Copy link
Contributor Author

brbrr commented Mar 8, 2018

Its not needed

@brbrr brbrr deleted the try/make-changeToNewRole-reliable branch March 8, 2018 08:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alisterscott alisterscott alisterscott left review comments

Assignees

@brbrr brbrr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brbrr @alisterscott